Skip to content

snap: Cleanup more environment variable set by launcher script #6969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kenvandine
Copy link
Contributor

This unsets a few more environment variables that were set by the launcher script which could potentially effect other apps.

@kenvandine kenvandine requested a review from a team as a code owner March 31, 2025 19:41
Comment on lines +2362 to +2379
env.remove("XDG_CONFIG_HOME");
env.remove("XDG_CACHE_HOME");
env.remove("XDG_DATA_HOME");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is safe. User or system could set this via systemd and wouldn't the launcher script inherit those paths?

Copy link
Contributor Author

@kenvandine kenvandine Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__

I'm not sure this is safe. User or system could set this via systemd and wouldn't the launcher script inherit those paths?

I think this should be fine. If a user is setting them in their own environment, they will get set when their shell starts. It's just environment variables that are set long enough to get ghostty running, before their shell starts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't set these variables in my shell init script. I set them in systemd.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not sure about this, because the user may not be launching a shell, they can launch any command that could rely on these env vars, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the blocker for this PR, any feedback?

Copy link
Member

@tristan957 tristan957 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your git history is messed up

@kenvandine
Copy link
Contributor Author

Your git history is messed up

Yeah, I already fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants